feat(app-server) add app server - #1803
Conversation
84d4ea0 to
c672900
Compare
limityan
left a comment
There was a problem hiding this comment.
PR Review: feat(app-server) add app server
This PR introduces a new bitfun-app-server interface crate providing a protocol-agnostic JSON-RPC server/client scaffold over agent_client_protocol custom roles, and rewires the Server Host WebSocket to use it (browser-direct ACP-over-WS). The overall architecture direction is sound: replacing the custom {type:"request"|...} WS envelope with native JSON-RPC 2.0, centralizing agent kernel + host service operations behind a single schema, and adding ts-rs generated TypeScript bindings. Tests are thorough (round-trip, agent kernel integration, event forwarding, websocket-adapter contract).
However, there are issues that should be addressed before merge:
Blocking
1. SubmitDialogTurnBody::to_request silently drops reply_route and prepended_reminders
schema.rs — The body struct accepts reply_route and prepended_reminders fields from the wire, but to_request() hardcodes them to None / Vec::new():
reply_route: None,
prepended_reminders: Vec::new(),This is silent data loss. If a client sends replyRoute or prependedReminders, they are accepted by deserialization then discarded. Either pass them through (reply_route: self.reply_route, prepended_reminders: self.prepended_reminders) or remove them from the wire body struct so the contract is honest.
2. Missing AGENTS-CN.md file
AGENTS.md line 1 has [中文](AGENTS-CN.md) but the PR does not create AGENTS-CN.md. This is a broken link. Either add the Chinese doc or remove the link.
Should fix
3. PR description is entirely blank
The PR body is the unfilled template — no Summary, Type, Areas, Motivation, Verification, or Reviewer Notes. Per the repo contribution process, at minimum the Verification section should record the commands run (e.g. cargo test -p bitfun-app-server, cargo check -p bitfun-server, pnpm run type-check:web, the websocket-adapter test).
4. log::info! in config handler should be debug or removed
server.rs — The config/getConfig handler logs every request at info level:
log::info!("server getConfig request: {:?}", request);This is noisy in production and logs request details (config paths). Per repo logging rules (src/crates/LOGGING.md), this should be log::debug! or removed.
5. #![recursion_limit] placement in test file is messy
tests/agent_kernel.rs — The #![recursion_limit = "512"] attribute is placed between //! doc comments, with // regular comments breaking the doc block:
//! ... matching `sdk_minimal.rs`. The
// Each `on_receive_request` ...
#![recursion_limit = "512"]
//! other operations ...
Move the attribute to the very top of the file (before all comments) for clarity.
6. gen:types in build scripts couples all web builds to Rust compilation
package.json — build, build:desktop, and build:web now all run npm run gen:types (which runs cargo test --features ts) before vite build. This means every frontend build requires a working Rust toolchain and compiles Rust code. For frontend-only iteration (pnpm run dev:web or CI), this could be a significant slowdown and adds a Rust dependency to the frontend build path. Consider making gen:types a separate opt-in step or only running it in CI/release builds, not every local build:web.
Consider
7. Event forwarder terminates connection on single notification send failure
server.rs serve main loop — If cx.send_notification(notification) returns Err, the loop returns Err, killing the entire connection including any in-flight RPCs. A single failed event push (e.g. transient backpressure) takes down the WebSocket. Consider logging and continuing instead of terminating, at least for non-fatal send errors.
8. {request} envelope unwrapping could misfire
const body = (params && typeof params === 'object' && 'request' in params)
? params.request ?? {}
: params ?? {};If a legitimate request body happens to have a top-level request field, it will be incorrectly unwrapped. The comment acknowledges this is transitional ("Step 2b"), but it's a footgun in the meantime. Consider checking for the exact {request: object} shape more narrowly, or documenting the risk.
9. recursion_limit bumps are a symptom, not a fix
#![recursion_limit = "256"] in lib.rs and main.rs, "512" in tests. Each new on_receive_request handler adds a ChainedHandler layer. As more host-service groups land (the PR comments mention workspace, snapshot, cron, MCP batches), this will need further bumps. This is a known trade-off of the builder pattern — worth tracking as tech debt.
10. Interfaces crate depends on assembly/core with product-full
app-server/Cargo.toml depends on bitfun-core with features = ["product-full"]. The PR acknowledges this mirrors bitfun-acp. It's a pre-existing pattern, not a new violation, but it means the interfaces layer pulls in the full product assembly graph. Worth noting for the architecture record.
Summary
The design is well-documented and the test coverage is good (round-trip, agent kernel, event forwarding, permission missing-port, client connect lifecycle regression). The main concern is the silent field dropping in to_request() (blocking) and the blank PR description. The rest are quality improvements that can follow.
79c38cb to
07c8da1
Compare
There was a problem hiding this comment.
复审结论:请求修改
整体方向与现有设计一致:Server/Web 应作为同级 adapter 复用唯一的 Agent Runtime,App Server 可以作为第一方 rich-client 私有协议;它不应成为第二套 Runtime 或公开 Agent SDK。当前实现主要偏离了既有设计中的“宿主负责协议转换、按能力垂直迁移、真实消费方与版本门槛”原则,合并前需处理以下问题。
1. [阻断] WebSocket 扩大到完整 Runtime 控制面,但没有认证和连接级作用域
问题: /ws 只检查可选 Origin,缺失 Origin 会直接放行;连接没有 initialize/token、workspace、用户或 execution-domain 身份。随后每个连接都能操作全局 Session、取消、权限、Git/config,并订阅同一全局事件与权限流。
风险: 任一本机非浏览器客户端都可直接调用完整控制面;多个 Web client 之间也无法证明 Session、事件和审批归属,可能看到或响应不属于自己的请求。旧 handler 仅暴露 ping/external-sources,本 PR 明显扩大了权限范围。
推荐方案: 在业务消息前完成 fail-closed 的初始化与认证,绑定 connection → user/workspace/session/execution-domain;事件和权限按连接作用域过滤,只允许控制该连接拥有的 Session/Turn。若本阶段只支持单用户本机模式,也应在设计中明确威胁模型和不可跨越的能力上限。
依据:docs/architecture/product-architecture.md:124-134、docs/architecture/agent-runtime-services-design.md:817-825;代码:src/apps/server/src/routes/websocket.rs:29-70、src/crates/interfaces/app-server/src/server.rs:428-546。
2. [阻断] Web adapter 只改方法名,没有完成请求/响应协议转换
问题: start_dialog_turn 仍发送 Desktop/Tauri 的 userInput/imageContexts 并期待 {success,message},Rust schema 要求 message/attachments 并返回 {status,sessionId,turnId}。权限接口还存在 requestId vs request_id、字符串 reply vs tagged object、workspaceId vs project_id;session/config/git 也有裸值与 {sessions}/{configs}/{branches} 包装不一致。适配器当前只解开 {request},没有 codec。
风险: Web 对话、权限审批、配置加载、Session 列表和 Git 分支等生产调用会反序列化失败或读到错误数据形状;绿色测试只验证方法名和独立 Rust schema,没有覆盖真实调用链。
推荐方案: 按命令提供明确的 request encoder / response decoder,或逐个把调用方迁到 owner DTO;增加 AgentAPI/PermissionAPI → WebSocket adapter → Rust schema 的端到端契约测试。不要一次性靠名称表宣称 Desktop/Web 已统一。
依据:docs/architecture/product-architecture.md:146-162;代码:src/web-ui/src/infrastructure/api/adapters/websocket-adapter.ts:38-105,366-385、src/crates/interfaces/app-server/src/schema.rs:86-109,179-279。
3. [阻断/设计偏离] app-server 边界过宽,并提前固化未版本化协议
问题: 新 crate 同时公开通用 role/transport、完整 agent + host-service schema、Rust client 和前端事件投影;其中 AppServerClient/FrontendEvent/connect 仅被测试使用,没有生产消费方。通用 client 内直接把现有 event_name + payload 投影成前端事件,协议没有 initialize/capability、版本、同流 sequence、调用关联、作用域或投递丢失语义。新增生命周期文档还把当前 preview Rust Runtime SDK 描述成“稳定 SDK 接口”。
风险: 单一 Server Host 的私有 DTO 会被过早冻结为通用 API,形成新的大而全后端 facade,并与 SDK Host/公开 Agent SDK 边界混淆;并发、断线和协议演进时,客户端无法识别事件缺口或兼容性。
推荐方案: 先收窄为当前 Server/Web 的真实垂直切片:宿主 DTO、前端事件投影留在 Server adapter;无真实消费方的通用 client/transport 保持内部或删除。为 Server/Web 单独定义版本化事件信封、capability 和丢失/恢复语义;将文档术语改回“Rust Runtime SDK(preview)”。
依据:docs/architecture/product-architecture.md:91-120,136-162、docs/architecture/agent-runtime-services-design.md:118-121,1031-1033、docs/architecture/agent-runtime-deployment-design.md:210-218;代码:src/crates/interfaces/app-server/src/lib.rs:1-14,47-74、src/crates/interfaces/app-server/src/client.rs:1-19,70-110。
4. [阻断] 传输迁移直接移除了现有 external-sources 能力
问题: 旧 Server Host dispatch 被断开,源码已明确说明相关命令现在进入 method_not_found;前端仍原样透传这些命令,没有 capability gate 或 typed unsupported。
风险: 这是已有 Web/Server 能力回归,用户只会得到通用 unknown-command 错误,也违反了逐能力迁移、无法等价时保留兼容边界的要求。
推荐方案: 在 app-server 对应垂直切片完整落地前保留旧 dispatch;或在本 PR 内迁移这些方法,并提供明确的 capability/unsupported 状态和 UI 门控。
依据:docs/architecture/product-architecture.md:106-120,156-162;代码:src/apps/server/src/routes/external_sources.rs:9-16、src/crates/interfaces/app-server/src/server.rs:422-425。
验证
当前 head 07c8da17edf9a237e605c4928367a59551d3c77e:7 项 CI、cargo check -p bitfun-server、cargo test -p bitfun-app-server、边界检查和 git diff --check 均通过;这些结果不能替代真实 Web 调用链、连接隔离和版本化事件契约验证。
b7a855a to
1bbc08b
Compare
limityan
left a comment
There was a problem hiding this comment.
整体方向中,复用现有 Agent Runtime、将浏览器通信收敛到 JSON-RPC/WebSocket 是合理的;但当前把传输适配、Server 运行时激活和全量后端 API 暴露放在一个 PR 中,架构边界和行为兼容性尚未闭合,建议修改后再合并。
1. Server 产品角色被静默改变
问题: src/apps/server/src/main.rs 现在无条件启动完整 Agent Runtime;这与当前架构文档及同文件中的只读 HTTP shell 约束冲突。实测 agent_bootstrap_reuses_core_ownership_without_activating_the_http_shell 失败。
风险: transport refactor 实际变成新的产品装配入口,后续容易与 Desktop 的生命周期、能力选择和资源所有权产生漂移。
建议: 本 PR 先保留窄幅、未激活的协议/router;将 Server delivery profile、运行时生命周期及架构文档更新拆成独立变更评审,不应仅删除失败测试。
2. 完整控制面缺少认证和作用域
问题: WebSocket 仅校验 Origin,没有连接 token、principal、workspace/execution-domain 绑定;所有连接还会接收全局 runtime/permission 事件。
风险: 本机进程可操作 session、权限、配置和 workspace,并可能跨项目观察或影响状态。
建议: 激活前增加安全握手、controller/observer 权限、workspace/execution-domain scope,并按 scope 过滤命令和事件。
3. 新 wire contract 尚未达到行为等价
问题: 旧 external-sources/dispatch 路径已退化为 method_not_found/unsupported;同时存在 dialogTurnId/turnId、图片 snake_case/camelCase、permission workspaceId/project_id 等真实字段错配。
风险: CI 和当前方法名映射测试可以通过,但生产调用会静默丢字段、取消错误 turn 或直接失去已有能力。
建议: 以 service-api -> WebSocket adapter -> Rust schema 的往返契约测试逐能力迁移;达到等价前保留旧路径,不要直接把内部 runtime DTO 当作公共 wire DTO。
4. 事件和接口边界还不具备远端契约
问题: broadcast lag/发送失败只记录后继续,没有 sequence、版本、恢复游标或 resync;app-server 同时依赖 product-full 并向下层 DTO 扩散 wire 类型派生。
风险: 漏掉 turn/permission 事件后客户端可能永久失同步,内部模型也会逐渐被公共协议反向约束。
建议: 定义版本化、可恢复、带 scope 的事件 envelope;将 app-server 收敛为小型 use-case API,只导出明确稳定的协议 DTO。
精确 head 1bbc08b 的 7 项 CI 当前均通过,但仓库自身的 Server 架构不变量测试可复现失败,因此绿色 CI 还不足以支持按当前形态合并。
8203cc1 to
274b81a
Compare
limityan
left a comment
There was a problem hiding this comment.
复核最新 head 0a8402e 后,仍有三个影响长期演进的阻断问题,建议修改后再合并。复用现有 Agent Runtime、收敛 JSON-RPC/WebSocket 的方向合理,以下只保留核心边界问题。
1. Server 仍成为第二个完整 Runtime 装配入口
问题: src/apps/server/src/main.rs 无条件启动完整 Agent Runtime,bootstrap.rs 又手工组装 Session、Execution、Coordinator、Scheduler、Cron 和全局服务;但权威架构及同文件约束仍规定当前 HTTP Server 不装配 Runtime。新增“旧 Server 已弃用、仅验证边界”的说明没有改变实际启动行为。
风险: Server 会形成独立的产品装配和生命周期入口,长期与 Desktop/CLI 的能力选择、ownership 和资源回收漂移;app-server -> product-full -> 全局服务 也容易演化成 service locator。
建议: 本 PR 保持 bootstrap dormant,只交付协议/transport 骨架;正式 Server Agent Host 通过单独评审的 Delivery Profile 和唯一 Assembly 构造入口落地。
2. 完整控制面仍只有 Origin 门禁
问题: WebSocket 没有认证、principal、workspace/user/execution-domain 绑定,但连接可以创建/取消 Session 和 Turn、答复权限、访问 Git 路径并修改全局配置。
风险: “连接即全权”一旦固化为协议事实,未来扩展 Remote、多 workspace 或多个控制端时很难兼容收紧,也无法可靠判定谁有权批准权限或取消执行。绑定 loopback 和严格 Origin allowlist 只能降低暴露面,不能替代身份与授权。
建议: 写操作启用前增加版本化初始化握手,绑定 principal、workspace/execution-domain、method capability 和 controller lease;取消、权限答复及配置写入按 Session/Turn ownership 校验。
3. 协议仍缺少版本、行为等价和恢复边界
问题: 最新提交只给 Rust 测试 fixture 补了 cancel_descendants: true,没有修复生产链路:前端仍发送 dialogTurnId,WebSocket adapter 未转换为协议要求的 turnId,Rust 会忽略未知字段并默认级联取消。事件 lag 也仍然只记录后继续,没有流失效或重同步信号。
风险: “取消指定 Turn”可能退化为取消当前活动 Turn并级联后代;事件丢失后客户端投影可能永久偏离权威状态。当前生成 DTO 和方法映射测试不足以证明跨 transport 行为等价。
建议: 增加 initialize/version/capability 协商;生产调用直接使用明确生成的 DTO;补 AgentAPI -> WebSocket adapter -> Rust handler 契约测试;事件丢失时 fail-closed 并要求从权威状态重建。
精确 head 的 7 项 CI 当前均通过,但这不能关闭上述 Runtime owner、安全作用域和协议演进问题。
当前web server本来就是不可用状态,先不考虑web server的功能正确性 |
limityan
left a comment
There was a problem hiding this comment.
已确认当前最新提交与 CI 状态,调整审查结论为通过。
Summary
Introduces a new bitfun-app-server interface crate — a protocol-agnostic JSON-RPC server/client scaffold built on agent_client_protocol custom roles (AppServer/AppClient) — and completes the Phase 2 wiring to expose agent-kernel operations and host services to the browser through this single generic surface. The Server Host's WebSocket handler is refactored to drive BitfunAppServer::serve directly via the new ws_transport::ws_lines adapter, replacing the old custom {type:"request"|"response"|"event"} envelope protocol. The browser now connects straight to the in-process app-server over native JSON-RPC 2.0 on the WebSocket — no shared in-process client, no hand-written command routing. The frontend websocket-adapter gains a typed AGENT_COMMAND_SCHEMA mapping that bridges the frontend's snake_case Tauri command names (start_dialog_turn, git_get_status, etc.) to the app-server's agent/, git/, config/* JSON-RPC method names, with request/response type slots generated from the Rust schema.rs via ts-rs.
Type and Areas
Type: Feature
Areas:
src/crates/interfaces/app-server (new crate)
src/apps/server (Server Host WebSocket refactor)
src/web-ui (websocket-adapter typed command mapping + ts-rs export)
src/crates/contracts (ts-rs ts feature additions for schema DTOs)
src/crates/assembly/core (CronService::new coordinator argument)
Motivation / Impact
The BitFun Server Host previously used a custom envelope protocol (WsMessage::Request/Response/Event) over the WebSocket, with a hand-written in-process handle_command router that only supported ping and external_sources. This left agent-kernel operations and host services inaccessible from web mode, and the server followed an entirely different code path from the Desktop host.
This PR unifies the desktop and web surfaces under a single app-server JSON-RPC interface. The browser-facing host interface is the same AgentRuntime SDK operations the Desktop host uses (create/list/delete sessions, submit/cancel dialog turns, permission reply/grants/audit) plus read-only git and config services — all over the same protocol transport. The generic role layer is schema-free, so the scaffold can be reused with a different schema in the future, while the agent/schema/server modules are the Phase 2 wiring that delegates to the bitfun_agent_runtime SDK and bitfun-core service singletons (Option C pattern, mirroring bitfun-acp).
Runtime events and permission lifecycle events are forwarded per-connection as agent/frontendEvent notifications, projected to the browser's existing agentic:// and permission://event channel names, so the frontend listen(...) call sites stay unchanged under browser-direct ACP.
Verification
The corresponding entries in the AGENTS.md verification table are "Shared Rust logic in
core,transport, adapters, or services" ->cargo check --workspaceplus the nearest focusedcargo test, and "Frontend UI, state, or adapters" ->pnpm run type-check:webplus the nearest focused test.Reviewer Notes
Checklist